node:http: closeIdleConnections() reads req.complete, and a destroyed request sets it - #44310

This pull request also holds the change of #44309, which is closed.

Repro of the sweep. Node v26.3.0 prints kept. Bun 1.4.3 and main print closed. This branch prints kept.

import { once } from "node:events";
import { createServer } from "node:http";
import { connect } from "node:net";
const server = createServer((req, res) => {
  res.end(req.url);
  if (req.url === "/sweep") server.closeIdleConnections();
});
server.keepAliveTimeout = 60000;
server.listen(0, "127.0.0.1");
await once(server, "listening");
const socket = connect(server.address().port, "127.0.0.1");
await once(socket, "connect");
socket.on("error", () => {});
let response = "";
const swept = Promise.withResolvers();
const kept = Promise.withResolvers();
socket.on("data", chunk => {
  response += chunk;
  if (response.endsWith("/sweep")) swept.resolve();
  if (response.endsWith("/second")) kept.resolve("kept");
});
const closed = once(socket, "close").then(() => "closed");
socket.write("GET /sweep HTTP/1.1\r\nHost: x\r\n\r\n");
await swept.promise;
socket.write("GET /second HTTP/1.1\r\nHost: x\r\n\r\n");
console.log(await Promise.race([closed, kept.promise]));
socket.destroy();
server.closeAllConnections();
server.close();

Repro of req.complete. The listener breaks out of a for await loop over the request after the first chunk and answers. The client then sends the rest of the body and one more request. Node v26.3.0 prints true. main prints false. This branch prints true.

import http from "node:http";
import net from "node:net";
import { once } from "node:events";
let first;
const server = http.createServer(async (req, res) => {
  if (req.url === "/second") return res.end(String(first.complete));
  first = req;
  for await (const chunk of req) break;
  res.end("first");
});
server.listen(0, "127.0.0.1");
await once(server, "listening");
const socket = net.connect(server.address().port, "127.0.0.1");
let received = "";
socket.on("data", chunk => (received += chunk));
socket.write("POST /first HTTP/1.1\r\nHost: x\r\nContent-Length: 10\r\n\r\nhello");
while (!received.endsWith("first")) await once(socket, "data");
socket.write("worldGET /second HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n");
await once(socket, "close");
console.log(received.slice(received.lastIndexOf("\r\n\r\n") + 4));
server.close();

What Node does.

• closeIdleConnections() destroys the connections of ConnectionsList::Idle(), which are the parsers with last_message_start_ == 0 (node_http_parser.cc). on_message_begin sets the field and on_message_complete clears it. The listener runs from on_headers_complete, between the two.
• parserOnMessageComplete (_http_common.js) sets stream.complete = true, adds the trailers and pushes the end of the stream. It does not look at destroyed.

Where the rule turns in Bun. _http_server.ts:1107 sets complete for a request without a body when the listener returns. The dispatch drains the ticks after that (src/runtime/server/mod.rs:1401 and :1564). So the listener sees false and its ticks see true, as in Node. For a request with a body, complete turns true one tick after the last chunk. No native state changes between the return of the listener and its ticks, so closeIfIdle() alone cannot tell the two apart. The sweep for connections that the JavaScript parser serves already works this way: closeIdleHttp1Connections (src/js/internal/http1_server_fallback.ts:709) skips a socket whose message is open.

Rows equal to Node v26.3.0. The table is in node-http-req-complete.test.ts, which runs under Bun and under Node: 6 request shapes by the places that can call the sweep, 64 rows. Each row is one keep-alive connection, one sweep, then one more request on that connection. Debug builds of the sources of main f4d755a and of this branch, Linux x64. The close() rows come from the same script with close() in the place of the sweep. That script is not in the tree.

The 4 rows that changed: the listener of a GET after res.end() (head in one read, head in two reads), and the 'data' listener of a Content-Length body (in the read of the head, in a later read). Node keeps, main closed, this branch keeps.

The 2 rows that still differ: a chunked body and a sweep from the 'end' listener of the request. Node closes, Bun keeps. hasIncompleteRequestBody() is still true in the last body callback, and a filter in JS cannot close a connection that closeIfIdle() keeps. Under Bun these 2 rows are todo tests.

One more difference that the table does not hold: the sweep from a microtask of the 'data' listener of the last chunk of a Content-Length body. Node keeps, main and this branch close. complete turns true in a nextTick there, and a nextTick runs before that microtask. From a nextTick of that listener this branch keeps, as Node does (main closes).

Requests of the same read. Three requests in one write, and the listener of the first calls the sweep. The responses that the client gets:

Bun runs the ticks of a listener before it parses the next request of the read, so a sweep from a tick closes the connection with /2 and /3 unread. That is the same on main. A fix needs a native close that waits until the read is consumed and then closes the connection if it is idle. HTTP_CLOSE_WHEN_IDLE is not that: nothing clears it, so it would also close a connection that is busy at the end of the read and idle much later. That change would also give close() in a listener a prompt close that drops no request. It is not in this pull request.

A destroyed request. The stream destroyer (src/js/internal/streams/destroy.ts) sets req.socket = null and calls destroy(), so the connection stays for the response. 8 public ways reach it for a server request: for await with a break, req.iterator() with a break, a pipeline() whose destination fails, a pipeline() whose generator returns early, promises.pipeline() with an aborted signal, Readable.toWeb(req) with cancel(), compose() and Duplex.from() with destroy(). With the rest of the body in one read, req.complete is true and req.trailers has the trailers in Node and on this branch for all 8. main has false and {} for all 8.

Node does not always get to the end of that message. Bun reads on in every case, as main does, so on this branch complete turns true whenever the body ends:

• The native server also calls ondata with the last-chunk flag when the connection closes (abort event). That call must not set complete. The two stays false tests cover it.
• A request that is pipelined behind a pending response gets complete and its trailers too (one test). The native wrapper that holds the armed callback stays alive through the server socket, which holds its current response and its queued responses.
• The sweep needs this part. Without it the connection of such a request keeps complete === false and the sweep never closes it.
• The cost for each read is what an unread body behind an ended response (req._dump()) costs on main: the native server makes a Buffer and calls ondata. Server CPU for the last 256 MiB of a body, about 515 reads, the client in another process, 5 rounds, interleaved:

Both rows pay for a Buffer that nobody reads. To remove that needs a change to the native body path. It is not in this pull request.

After close(). The sweep of a server that close() stopped is the same as on main: it asks closeIfIdle() about each connection, also the one in dispatch. An earlier version of this change used Node's rule there too. Then a shutdown handler in the order close(cb), res.end(), closeIdleConnections() waited for the keepAliveTimeout. Time from the listener to the close callback, one client that stays connected:

close() is not in this change. The first version gave close() the rule of the sweep. Then res.end(); server.close(cb) in a listener calls cb when the client closes the connection or the keepAliveTimeout ends it, as in Node v26.3.0. Measured with that version: 4006 ms with the default agent and 6002 ms with a keep-alive agent (Node: 4009 ms and 6007 ms, main: 2 ms).

• node:http: the server follows Node (request body, framing, response finish, lifecycle) #43557 recorded this as a known difference that it left: "A handler that calls res.end() and then server.close() closes its keep-alive connection at once. Node waits for the keepAliveTimeout."
• http: close connections that go idle after server.close() nodejs/node#66000 (open, approved) marks the server as closing in close(). A connection then closes when its response finishes with nothing queued behind it.
• close() still sweeps natively, so it sees the connections of the current listener only.

bun --hot. A reload runs the script again. Its listen() gets the native server of the last run back from Bun.serve(), with the connections of that run, and a new http.Server has an empty set of connections. The sweep of main is native and sees every connection. The walk sees only the set. So listen() now moves the connections of the http.Server that had this native server to the new one. One keep-alive connection from before the reload and one new connection:

The --hot test also reads getConnections() after the sweep closed that connection: its close is counted down on the new server.

What reads socket.server or the set now gets the new server for such a connection: 'timeout' and 'clientError' go to the new server, its keepAliveTimeout, headersTimeout, requestTimeout and maxConnections apply, and its 'close' waits for the connection. Outside bun --hot, Bun.serve() never gives a native server back, and listen() pays one WeakMap read and one write.

Other behaviour that changes with the walk.

• closeIdleConnections() also closes the idle connection of an earlier listen() of the same server. The set of connections belongs to the server, as the list in Node does. Before, the sweep of a listening server saw only the connections of the current listener.
• A response that the socket did not take whole, closeIdleConnections() in its listener: the connection stays open behind the response. main closed it when the response had left.
• After close(), the sweep closes the newest connection first, as the sweep of a listening server does. Before, it closed the oldest first there. Node orders idle parsers by address.

Related. #37889 (open since August, conflicts with main) gives both methods Node's rule in uWS. #35839 (closed when #43557 merged) had the walk over the connections of the server, with a native key that is still set while the ticks of the dispatch run.

A socket that a program emits as a 'connection' of a second http.Server is counted down on that second server when it closes, because #onClose finds the set through the public socket.server. Then getConnections() of the first server stays 1 and its close(cb) does not call back (Node v26.3.0: 0, and it calls back). That is the same on main, and this change does not touch it. #42594 keeps the accepting server in a field of the socket. The --hot loop of this change must then set that field too: with a field that only the constructor sets, the --hot test fails with left 1.

Found outside this change.

• Readable.toWeb(req).cancel() in a listener, then res.end() in the same listener: emitResponseFinish throws TypeError: undefined is not an object from onResponseFinishHandleSocket, on main and on this branch. req.socket is null there, the dispatcher has detached the response, and res.socket then makes a FakeSocket that has no server. The tests here end that response behind an await. A search found no open pull request for it.
• expect(promise).rejects on a pending promise, called from the continuation of a socket event, can wait for ever on Windows. See Windows below.

Review.

The first self-review had 4 concerns. All were about close(), which this change now leaves alone. The review of the two stacked pull requests found 2 cases that were worse than main (the order close, end, sweep, and bun --hot) and 4 points in the _destroy part (a dead clause, the trailers, test waits that could hang, the cost for each read). The first 5 are fixed. The cost is measured above.

The self-review of this diff raised 21 concerns.

• Addressed, 13: the stale comment on armed_this_value. The table of rows now runs under Node in the tree. The destroy paths were run by hand (the 8 ways above, 3 test files of test/js/node/http and 5 vendored stream tests on both builds). The --hot test also reads socket.server. The text no longer claims that Node always completes a destroyed request. 9 tests that only repeated rows of the table are gone. New tests: a pipelined destroyed request, a request that a cancelled web stream destroyed in the listener, the rest of a body in two reads. Two comment groups are one line now.
• Not taken, 6. The rule stays in JS and not in closeIfIdle(): see Where the rule turns in Bun. A sweep from a tick, close() in a listener and a sweep after close() still drop requests of the same read: see Requests of the same read. close() does not use the walk. The microtask row above still differs.
• No change needed, 2: this stays one pull request, and the gate in JS is not traded for a native mark.

The review of this pull request after its first push: the comment on armed_this_value is 3 lines again, the --hot test has no timeout of its own, and the 2 rows where Bun differs are todo tests. Not taken, with the reason in each thread: a guard in _destroy for a destroy between the last chunk and the tick that completes the request (9 tries did not reach that state), a refresh of the inactivity timer for the rest of a destroyed body (Node closes that connection too), and lenient parsing of these trailers (the expression is the one for a request that ended normally).

Measurements. Release builds of main bf42a52 and of this branch before the last merges of main, Linux x64. The source under src/ differs from those builds in comments only. perf, valgrind, strace and bloaty are not installed on the machine, so instruction counts are missing. The machine had other load, so the wall-clock numbers carry their range.

Suites. Debug build of this branch: node-http-server-close-drain.test.ts (47 pass), node-http-req-complete.test.ts (42 pass and 2 todo, and 41 pass under Node v26.3.0), node-http-server-abort-events.test.ts, node-http-req-socket-pause.test.ts, node-http-server-timeouts.test.ts, and the vendored test-stream-pipeline.js, test-stream-finished.js, test-stream-readable-async-iterators.js, test-webstreams-pipeline.js, test-stream-pipeline-http2.js. With the sources of main, 25 of the 89 tests of the first two files fail and the rest of this list passes. The two files ran again after the merge of main b28212a, with the same counts. Release builds of main bf42a52 and of an earlier head of this branch, one after the other: the 38 test files of test/js/node/http, node-http2.test.js, bun-server.test.ts, serve.test.ts (1,916 test cases), 499 vendored Node tests (test-http*, test-https*) and the 67 scripts of test/js/bun/test/parallel. No test changed its result except the new ones. 11 of the scripts and 2 cases of serve.test.ts fail on both builds on this machine. On this machine, under heavy load from other work, tests of node-http-server-close-drain.test.ts that are not in this diff sometimes pass the 5 s limit of a local run.

Windows. Not run locally. In CI, an earlier form of one new test timed out on both Windows lanes and passed on Linux and macOS. That test called expect(promise).rejects on a pending promise, from the continuation of a socket 'data' event. expect().rejects waits in a nested event loop (wait_for_promise, src/runtime/test_runner/expect.rs:496). The event that settles the promise was the close of a second client socket. Read from the code, not confirmed on a Windows machine: libuv takes a batch of completions from the IOCP and processes it one by one (uv__process_reqs), so a completion from the same batch as the 'data' event waits for that callback to return, and the nested loop never gets it. epoll and kqueue report the socket again. The tests no longer wait this way. They send one more request on each connection and read answered or closed, as the tests of #44210 do.

Observed on Linux x64 only.